Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script templates #1

Merged
merged 11 commits into from
Feb 9, 2023
Merged

Script templates #1

merged 11 commits into from
Feb 9, 2023

Conversation

mt-edwards
Copy link
Contributor

Added Python and R template to the project template. You can now select the language of choice.

Comment on lines 33 to 35
├─ model/
├─ raw/
├─ wrangle/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be useful to re-order these in terms of the actual order of the workflow i.e. raw -> wrangle -> model ?

@@ -9,7 +9,7 @@ pip install cookiecutter
cookiecutter https://github.com/NICD-UK/project-template
```

You will be prompted for nine inputs:
You will be prompted for eleven inputs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider changing "eleven" to "the following"

Copy link
Contributor

@m-misiura m-misiura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all 16 files:

  • pyprojroot appears to be the cleanest solution to the problem of specifying paths without ../
  • the flow from raw -> clean -> wrangle appears to be fine for both Python and R files
  • is the file {{cookiecutter.project_directory_name}}/notebooks/.gitkeep needed ? it's an empty file
  • there are some very minor comments on the README.md

Overall, great job !

@m-misiura m-misiura merged commit ea7f7f4 into main Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants